-
Notifications
You must be signed in to change notification settings - Fork 200
OCaml 5.4 syntax support #2720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
OCaml 5.4 syntax support #2720
Conversation
- update vendored parsers to mirror upstream at 5.4: * introduce locations for Longident.t components * distinguish (module M:S) and ((module M):(module S)) for expressions - support for new syntaxes: * bivariance * labelled tuples
eabfcee
to
201949e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the slow reply. This is awesome :) Thanks a lot!
| Ptyp_tuple of core_type list | ||
(** [Ptyp_tuple([T1 ; ... ; Tn])] | ||
represents a product type [T1 * ... * Tn]. | ||
| Ptyp_tuple of (string option * core_type) list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to attach a location to the label to support comments around them.
@@ -3185,10 +3316,12 @@ type_variance: | |||
| MINUS BANG { [ mkvarinj "-" $loc($1); mkvarinj "!" $loc($2) ] } | |||
| BANG MINUS { [ mkvarinj "!" $loc($1); mkvarinj "-" $loc($2) ] } | |||
| INFIXOP2 | |||
{ if ($1 = "+!") || ($1 = "-!") then [ mkvarinj $1 $sloc ] | |||
{ if $1 = "+!" || $1 = "-!" || $1 = "+-"|| $1 = "-+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could accept anything starting in +
, -
or !
here ? The standard parser will catch errors there anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the parser point of view, we could accept any INFIXOP2. And I agree that it probably better to accept this when building the CST.
@@ -2822,13 +2863,36 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens | |||
in | |||
let outer_wrap = has_attr && parens in | |||
let inner_wrap = has_attr || parens in | |||
let with_label (lbl, exp) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead handling special cases for ( ~foo, ~(bar : t) )
, etc.. do you think we could represent the concrete syntax in the parsetree ?
I mean something like type tuple_label = Tl_pun of string loc | Tl_constraint of string loc * core_type | Tl_normal of string loc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are case where people don't want to normalize ~x:x, ~y
to ~x, ~y
? If yes, it is probably better to separate the two forms in the parsetree. Otherwise, I am not sure if this is that useful?
This PR updates the vendored parsers and related types to mirror the 5.4 version of the OCaml parser and adds a basic support for labeled tuples and bivariance.
The parser updates means that it is possible to locate comments between
Longident.t
component, and it is always possible to distinguish between(module M): (module S)
at least for expressions.The support for labeled expressions is bit ad-hoc and hackish in this version due to the way that labeled tuple element interact with parenthesis, for instance inside:
and it could definitively be improved.